Skip to content

fix: drop RegExp.exec, negation and test-helper false positives - #2

Merged
Revinand merged 4 commits into
masterfrom
fix/false-positives-regexp-negation-test-helpers
Nov 15, 2026
Merged

Revinand merged 4 commits into
masterfrom
fix/false-positives-regexp-negation-test-helpers

Conversation

@SergeevDmitry

@SergeevDmitry SergeevDmitry commented Nov 15, 2026 •

Copy link
Copy Markdown
Member

Summary

Three false positives found by scanning the modelcontextprotocol/servers reference servers in source mode.

Taint: RegExp.prototype.exec reported as command injection

The bare exec sink, meant for import { exec } from "child_process", matched any X.exec(...). In the filesystem server, WINDOWS_DRIVE_PREFIX.exec(path) and WINDOWS_UNC_ROOT.exec(path) were HIGH command injection. A JavaScript/TypeScript receiver now skips the sink only when it is a regex literal, new RegExp(...), RegExp(...), or a name the file binds only to one of these. A name assigned anything else, or shadowed by a parameter, loop variable or catch binding, keeps the sink.

Behavioral: negation read at the wrong occurrence

negatedNear looked for a negator before the first substring match of the word (strings.Cut), while the word itself was matched on word boundaries. "api" was looked up inside "rapid", and only the first occurrence of a word in a sentence was checked. The fetch server's description ("...you did not have internet access, ... this tool now grants you internet access") was reported as behavioral.contradicted_network at HIGH. Each whole-word occurrence is now judged at its own position, and a word that the same sentence both negates and uses plainly is not a denial. Other words of the category do not lift a denial: "never contacts the network or any web api" still denies the network.

Source: a local function named tool registered as an MCP tool

In a tree that imports an MCP SDK, a receiver-less tool(a, b, ...) call counted as a registration. The fetch server's test helper tool(bindir, "node", body) produced 16 tools named node, npm and reader, which behavioral, taint and yara then reported as discovered but not analyzed. A receiver-less call that resolves to the project's own function and passes no function argument is no longer a registration. The call still counts when the calling file imports that name from a package, or when it passes an inline function or a function defined in the tree, the shape of a hand-rolled registry such as tool("read_note", "...", async (args) => ...). Calls through a receiver (server.tool(...)) are unchanged.

Results

Source scans of modelcontextprotocol/servers at e1dc4b1:

Server Before After
filesystem 4 high, 11 medium, 6 low 2 high, 11 medium, 6 low
fetch 1 high; 17 tools; 7 coverage gaps no findings; 1 tool; 4 coverage gaps
everything, git, memory, sequentialthinking, time unchanged

Behaviour changes

Fewer taint.command_injection findings on regex .exec calls and fewer behavioral.contradicted_* findings on descriptions that use the negated word plainly in the same sentence. Trees with a local function named tool, prompt or resource report fewer capabilities and so fewer coverage gaps.

Notes

internal/conventions failed on master since 1c22b1d: two comments named the placeholder path docs/release-notes/TAG.md and nothing linked docs/release-notes/v1.1.0.md. The last commit writes the placeholder as <TAG> and links the v1.1.0 notes from the README documentation list.

@Revinand
Revinand merged commit 97e3b6e into master Nov 15, 2026
2 checks passed
@Revinand
Revinand deleted the fix/false-positives-regexp-negation-test-helpers branch November 15, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants